Skip to content

Combining multiple object files in JIT #2530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Draft
ypapadop-amd wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Combining multiple object files in JIT #2530

ypapadop-amd wants to merge 3 commits into from

Conversation

@ypapadop-amd
Copy link
Collaborator

@ypapadop-amd ypapadop-amd commented Aug 22, 2025

No description provided.

@ypapadop-amd ypapadop-amd mentioned this pull request Aug 22, 2025
Open
mawad-amd
mawad-amd reviewed Aug 23, 2025
View reviewed changes
Copy link
Collaborator

@mawad-amd mawad-amd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for writing this. This approach looks fine to me and tying the code object name to the kernel itself is good enough for me for now. The non-ideal thing here is that a user must have one function per source file. It's not ideal but a non-issue for me. I will let Jeff and Joe decide if that's an issue.

I ported the tests I wrote to have each function in different file here if you wanted to reuse them https://gist.github.com/mawad-amd/56fed24599343f22ab4f51d0c679a4a9

@ypapadop-amd
Copy link
Collaborator Author

ypapadop-amd commented Aug 24, 2025

Thanks for writing this. This approach looks fine to me and tying the code object name to the kernel itself is good enough for me for now. The non-ideal thing here is that a user must have one function per source file. It's not ideal but a non-issue for me. I will let Jeff and Joe decide if that's an issue.

I ported the tests I wrote to have each function in different file here if you wanted to reuse them https://gist.github.com/mawad-amd/56fed24599343f22ab4f51d0c679a4a9

That is true, you can't have more than one function referenced from an ExternalFunction in a single compilation unit as the current ExternalFunction design has 1:1 map between external function and compilation units. This is only a prototype to allow the use-case that was brought up.

I don't have a good solution for having multiple ExternalFunctions to bind to the same object file. Maybe we allow one ExternalFunction to bind to another one? Or create an array of ExternalFunctions?

@ypapadop-amd ypapadop-amd force-pushed the ypapadop-amd/combine-external-function-objects branch from 88a0a36 to 6924e03 Compare August 26, 2025 12:02
@ypapadop-amd ypapadop-amd mentioned this pull request Aug 27, 2025
Open
@ypapadop-amd ypapadop-amd force-pushed the ypapadop-amd/combine-external-function-objects branch from 6924e03 to 5dea96a Compare August 27, 2025 15:27
@ypapadop-amd ypapadop-amd force-pushed the ypapadop-amd/combine-external-function-objects branch from 5dea96a to 3dcca27 Compare September 3, 2025 19:54
@ypapadop-amd ypapadop-amd force-pushed the ypapadop-amd/combine-external-function-objects branch from 3dcca27 to cab831f Compare September 25, 2025 13:49
@ypapadop-amd ypapadop-amd force-pushed the ypapadop-amd/combine-external-function-objects branch from cab831f to 01e9e29 Compare October 8, 2025 16:58
@hunhoffe hunhoffe added this to the JIT/Runtime/Compilation Refactor milestone Nov 10, 2025
@hunhoffe hunhoffe mentioned this pull request Nov 19, 2025
Open
23 tasks
@hunhoffe hunhoffe removed this from the JIT/Runtime/Compilation Refactor milestone Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mawad-amd mawad-amd mawad-amd left review comments

@AndraBisca AndraBisca Awaiting requested review from AndraBisca AndraBisca will be requested when the pull request is marked ready for review AndraBisca is a code owner

@jackl-xilinx jackl-xilinx Awaiting requested review from jackl-xilinx jackl-xilinx will be requested when the pull request is marked ready for review jackl-xilinx is a code owner

@andrej andrej Awaiting requested review from andrej andrej will be requested when the pull request is marked ready for review andrej is a code owner

@pvasireddy-amd pvasireddy-amd Awaiting requested review from pvasireddy-amd pvasireddy-amd will be requested when the pull request is marked ready for review pvasireddy-amd is a code owner

@jgmelber jgmelber Awaiting requested review from jgmelber jgmelber will be requested when the pull request is marked ready for review jgmelber is a code owner

@hunhoffe hunhoffe Awaiting requested review from hunhoffe hunhoffe will be requested when the pull request is marked ready for review hunhoffe is a code owner

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ypapadop-amd @mawad-amd @hunhoffe